Skip to content

Add logging to TAP file#94

Merged
mblayman merged 16 commits into
python-tap:mainfrom
codambro:add-logging
Jan 30, 2025
Merged

Add logging to TAP file#94
mblayman merged 16 commits into
python-tap:mainfrom
codambro:add-logging

Conversation

@codambro

@codambro codambro commented Jan 24, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

resolves #91

This is based off the existing pytest JUnit logging support.

--tap-logging 'all'

1..6
ok 1 test_logging.py::test_ok
not ok 2 test_logging.py::test_not_ok
# def test_not_ok():
#         LOGGER.error("Running test_not_ok")
# >       assert False
# E       assert False
# 
# test_logging.py:12: AssertionError
# --- Captured Log ---
# ERROR    test_logging:test_logging.py:11 Running test_not_ok
# --- Captured Out ---
# 
# --- Captured Err ---
#
ok 3 test_logging.py::test_params[foo]
ok 4 test_logging.py::test_params[bar]

NOTE: Have PR to add logging to passing tests, but that requires an update to tappy.
tappy PR - python-tap/tappy#148
pytest-tap PR - codambro#1

To accept your contribution, please complete the checklist below.

  • Is your name/identity in the AUTHORS file alphabetically?
  • Did you write a test to verify your code change?
  • Is CI passing?

@codambro

codambro commented Jan 28, 2025

Copy link
Copy Markdown
Contributor Author

@mblayman Could I get a review on this? I can't seem to request reviewers on this repo

mblayman
mblayman requested changes Jan 29, 2025
View reviewed changes

@mblayman mblayman left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks pretty decent to me. Config options are forever, practically, so I'd like to avoid adding one of these.

Comment thread src/pytest_tap/plugin.py Outdated
Comment thread src/pytest_tap/plugin.py
Comment thread src/pytest_tap/plugin.py Outdated
@codambro codambro requested a review from mblayman January 29, 2025 14:59
@codambro

codambro commented Jan 29, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Believe it is ready to re-review. Some concern as the default behavior in pytest is for --show-capture=all. Meaning anybody using pytest-tap will now by default have log/out/err all added to failing test diagnostics. I don't know if there is a case where a user may want the TAP result file to be kept minimal, whereas they have a separate log file with all the test output. By combining it with --show-capture, they no longer can separate them.

I'm also considering to make these ini config options (via addini) as opposed to command line args. Since that's how JUnit result files are configured.

@mblayman

mblayman commented Jan 30, 2025

Copy link
Copy Markdown
Member

Some concern as the default behavior in pytest is for --show-capture=all.

I'm not concerned. The library makes no promises about what diagnostics it outputs. This configuration lines up with the default pytest behavior, which feels good to me.

mblayman
mblayman approved these changes Jan 30, 2025
View reviewed changes

@mblayman mblayman left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution! 👍

@mblayman mblayman merged commit e8f9c78 into python-tap:main Jan 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mblayman mblayman mblayman approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Allow additional output

2 participants

@codambro @mblayman